8362376: Use @Stable annotation in Java FDLIBM implementation #26341

jddarcy · 2025-07-16T04:53:04Z

Add @Stable to the static final arrays used in the Java port of FDLIBM.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8362376: Use @Stable annotation in Java FDLIBM implementation (Enhancement - P4)

Reviewers

Raffaello Giulietti (@rgiulietti - Reviewer)
Chen Liang (@liach - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26341/head:pull/26341
$ git checkout pull/26341

Update a local copy of the PR:
$ git checkout pull/26341
$ git pull https://git.openjdk.org/jdk.git pull/26341/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26341

View PR using the GUI difftool:
$ git pr show -t 26341

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26341.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-07-16T04:53:36Z

👋 Welcome back darcy! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-07-16T04:53:51Z

@jddarcy This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8362376: Use @Stable annotation in Java FDLIBM implementation

Reviewed-by: rgiulietti, liach

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 69 new commits pushed to the master branch:

7da274d: 8361961: Typo in ProtectionDomain.implies
4e0b035: 8338282: javax/swing/JMenuBar/TestMenuMnemonicLinuxAndMac.java test failed on macOS and Ubuntu
a23987f: 8361283: [Accessibility,macOS,VoiceOver] VoiceOver announced Tab items of JTabbedPane as RadioButton on macOS
... and 66 more: https://git.openjdk.org/jdk/compare/0acd065bf5a75090b84c28b28856a62d86c52791...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

jddarcy · 2025-07-16T04:54:25Z

Some small refactorings could be used to make a few non-static arrays static.

openjdk · 2025-07-16T04:54:35Z

@jddarcy The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-07-16T04:57:48Z

Webrevs

rose00 · 2025-07-16T18:32:13Z

The effect of this PR is to make the affected array elements eligible for constant-folding by the JIT.

The contract of @Stable is private to the JDK, since it is a trusted annotation. The trusted user promises not to change the array elements after the JIT might have observed them.

If we had a frozen (immutable) array feature in the JVM this PR could be formulated using frozen array constants. But we are not there yet.

jddarcy · 2025-07-16T18:44:05Z

The effect of this PR is to make the affected array elements eligible for constant-folding by the JIT.

The contract of @Stable is private to the JDK, since it is a trusted annotation. The trusted user promises not to change the array elements after the JIT might have observed them.

If we had a frozen (immutable) array feature in the JVM this PR could be formulated using frozen array constants. But we are not there yet.

Yes; my intention was to allow HotSpot greater leeway to optimize the FDLIBM code. Under my limited performance testing, the change seemed to be performance neutral, but if it shouldn't cause a regression, I'd be comfortable pushing the change.

jddarcy · 2025-07-16T18:51:41Z

The methods directly affected by this update are atan, exp, and sin, cos, tan. The sin, cos, and tan method are affected by the updates to KernelRemPio2 and tan is also directly affected by its own updates.

rgiulietti

The arrays at L.2257-2262 could be declared static and @Stable as well, and moved outside the method.

rgiulietti · 2025-07-17T10:27:27Z

src/java.base/share/classes/java/lang/FdLibm.java

@@ -809,6 +814,7 @@ static final class KernelRemPio2 {

        private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk


Suggested change

private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk

@Stable private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk

Suggested change

private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk

@Stable

private static final int[] init_jk = {2, 3, 4, 6}; // initial value for jk

Missed that array on my edit; will add. Thanks.

wenshao · 2025-07-17T10:50:05Z

src/java.base/share/classes/java/lang/FdLibm.java

@@ -451,7 +453,8 @@ static double compute(double x) {
         */
        private static final double
            pio4  =  0x1.921fb54442d18p-1,  // 7.85398163397448278999e-01
-            pio4lo=  0x1.1a62633145c07p-55, // 3.06161699786838301793e-17
+            pio4lo=  0x1.1a62633145c07p-55; // 3.06161699786838301793e-17
+        @Stable private static final double


Suggested change

@Stable private static final double

@Stable

private static final double

liach

These additions look reasonable.

rgiulietti · 2025-07-17T18:04:31Z

@jddarcy What about the local arrays mentioned in my comment above

The arrays at L.2257-2262 could be declared static and @Stable as well, and moved outside the method.

liach · 2025-07-17T18:16:33Z

I think local arrays require more structural changes to the code, which might be why this patch did not include that change. However, note as said in #26355, such conversions are meaningful and would potentially allow performance boosts.

jddarcy · 2025-07-17T21:09:20Z

The arrays at L.2257-2262 could be declared static and @Stable as well, and moved outside the method.

Right; those were the ones I was referring to when I wrote "Some small refactorings could be used to make a few non-static arrays static." :-)

I wanted to just start with the static final arrays, but I could pull those out too in this PR.

jddarcy · 2025-07-17T21:36:19Z

I think local arrays require more structural changes to the code, which might be why this patch did not include that change. However, note as said in #26355, such conversions are meaningful and would potentially allow performance boosts.

Okay; I took a look at what the code was actually doing and wrote something that avoiding using arrays entirely.

rgiulietti

Looks fine

liach

This array unrolling looks right to me.

jddarcy · 2025-07-18T16:54:36Z

This array unrolling looks right to me.

I'll write a regression test case to make sure.

rgiulietti · 2025-07-18T19:58:18Z

Since the refactoring is rather simple, I'm not sure "yet another test" is really needed. But if you are unsure, then please go ahead.

ExE-Boss · 2025-07-20T06:29:18Z

src/java.base/share/classes/java/lang/FdLibm.java

+        @Stable
+        private static final double
            T[] = {


The legacy C‑like array field syntax is confusing in Java, and it’s not used for the @Stable int[] arrays below.

Suggested change

@Stable

private static final double

T[] = {

@Stable

private static final double[] T = {

Or at least put the T[] on the same line so that it’s clear why this field has @Stable, since regular static final doubles are already constant folded and @wenshao’s comment¹ confused me on the “Conversation” tab, since it was missing the T[] there.

Suggested change

@Stable

private static final double

T[] = {

@Stable

private static final double T[] = {

Footnotes

https://github.com/openjdk/jdk/pull/26341#discussion_r2213012163 ↩

turbanoff · 2025-07-22T11:00:19Z

src/java.base/share/classes/java/lang/FdLibm.java

+                //                       1.5};
+                // final double DP_H[]  = {0.0,
+                //                        0x1.2b80_34p-1};        // 5.84962487220764160156e-01
+                final double DP_H1   = 0x1.2b80_34p-1;


Suggested change

final double DP_H1 = 0x1.2b80_34p-1;

final double DP_H1 = 0x1.2b80_34p-1;

liach · 2025-07-22T15:20:24Z

Re @turbanoff and @ExE-Boss's style fixes: this code is ported from the c code of fdlibm so it has a lot of irregular styles. I think we might look into updating the implementation to be more Java-ish in the future in another patch.

jddarcy · 2025-07-22T16:35:22Z

Re @turbanoff and @ExE-Boss's style fixes: this code is ported from the c code of fdlibm so it has a lot of irregular styles. I think we might look into updating the implementation to be more Java-ish in the future in another patch.

I'll address the array declaration syntax in the next commit on the PR. While the current state of Fdlibm.java does show its original C lineage, it has already gone through a few passes of getting it closer to idiomatic Java.

JDK-8362376: Use @stable annotation in Java FDLIBM implementation

7fec151

openjdk bot changed the title ~~JDK-8362376: Use @Stable annotation in Java FDLIBM implementation~~ 8362376: Use @Stable annotation in Java FDLIBM implementation Jul 16, 2025

openjdk bot added the rfr Pull request is ready for review label Jul 16, 2025

openjdk bot added the core-libs core-libs-dev@openjdk.org label Jul 16, 2025

jddarcy marked this pull request as draft July 16, 2025 05:00

openjdk bot removed the rfr Pull request is ready for review label Jul 16, 2025

jddarcy marked this pull request as ready for review July 16, 2025 17:47

openjdk bot added the rfr Pull request is ready for review label Jul 16, 2025

rgiulietti reviewed Jul 17, 2025

View reviewed changes

wenshao reviewed Jul 17, 2025

View reviewed changes

Respond to review feedback and update copyright.

a7b86ee

liach approved these changes Jul 17, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 17, 2025

Implement review feedback.

5e1233b

openjdk bot removed the ready Pull request is ready to be integrated label Jul 17, 2025

rgiulietti approved these changes Jul 18, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 18, 2025

liach approved these changes Jul 18, 2025

View reviewed changes

liach mentioned this pull request Jul 20, 2025

8357913: Add @Stable to BigInteger and BigDecimal #25437

Closed

3 tasks

ExE-Boss reviewed Jul 20, 2025

View reviewed changes

turbanoff reviewed Jul 22, 2025

View reviewed changes

		@@ -809,6 +814,7 @@ static final class KernelRemPio2 {

		private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk

	private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk
	@Stable private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk

	private static final int init_jk[] = {2, 3, 4, 6}; // initial value for jk
	@Stable
	private static final int[] init_jk = {2, 3, 4, 6}; // initial value for jk

	@Stable private static final double
	@Stable
	private static final double

	final double DP_H1 = 0x1.2b80_34p-1;
	final double DP_H1 = 0x1.2b80_34p-1;

8362376: Use @Stable annotation in Java FDLIBM implementation #26341

Are you sure you want to change the base?

8362376: Use @Stable annotation in Java FDLIBM implementation #26341

Conversation

jddarcy commented Jul 16, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jul 16, 2025

Uh oh!

openjdk bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jddarcy commented Jul 16, 2025

Uh oh!

openjdk bot commented Jul 16, 2025

Uh oh!

mlbridge bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

rose00 commented Jul 16, 2025

Uh oh!

jddarcy commented Jul 16, 2025

Uh oh!

jddarcy commented Jul 16, 2025

Uh oh!

rgiulietti left a comment

Choose a reason for hiding this comment

Uh oh!

rgiulietti Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

wenshao Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

jddarcy Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

wenshao Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

rgiulietti commented Jul 17, 2025

Uh oh!

liach commented Jul 17, 2025

Uh oh!

jddarcy commented Jul 17, 2025

Uh oh!

jddarcy commented Jul 17, 2025

Uh oh!

rgiulietti left a comment

Choose a reason for hiding this comment

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

jddarcy commented Jul 18, 2025

Uh oh!

rgiulietti commented Jul 18, 2025

Uh oh!

ExE-Boss Jul 20, 2025

Choose a reason for hiding this comment

Footnotes

Uh oh!

turbanoff Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

liach commented Jul 22, 2025

Uh oh!

jddarcy commented Jul 22, 2025

Uh oh!

Uh oh!

jddarcy commented Jul 16, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jul 16, 2025 •

edited

Loading

mlbridge bot commented Jul 16, 2025 •

edited

Loading